-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use NullArray to Pass row count to ScalarFunctions that take 0 arguments #328
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW I think this looks good, and is the consensus direction for this feature:
(see #303 (comment))
The tests for random()
will cover the code in this PR I think.
I had one comment regarding From
vs TryFrom
but then I think it is ready
a3c7acc
to
efcdd4a
Compare
Codecov Report
@@ Coverage Diff @@
## master #328 +/- ##
=======================================
Coverage 75.72% 75.72%
=======================================
Files 143 143
Lines 23832 23840 +8
=======================================
+ Hits 18046 18054 +8
Misses 5786 5786
Continue to review full report at Codecov.
|
impl BuiltinScalarFunction { | ||
/// an allowlist of functions to take zero arguments, so that they will get special treatment | ||
/// while executing. | ||
fn supports_zero_argument(&self) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
had to add this whitelist because vararg funcs like array
will not accept 0 args
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @jimexist !
I agree -- thanks again @jimexist -- great stuff |
Which issue does this PR close?
fix 305 by using a null array as param for zero param functions
Closes #305
Rationale for this change
in case of a zero param function we'll have a rather hacky way to pass in the batch num_rows param so that the functions wrapped can be generative the correct result
What changes are included in this PR?
Similar to #307 but use a null array instead
Are there any user-facing changes?